Skip to content

Update Leaders donation limit text #2191

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Update Leaders donation limit text #2191

wants to merge 3 commits into from

Conversation

house-ben
Copy link

@house-ben house-ben commented Sep 11, 2025 •
edited
Loading

Fixed the contribution description for #1766

Since the heroes are based on the total donated_amount contributed, not the donations over the past year, the text has to be updated.

djangoproject.com/fundraising/templatetags/fundraising_extras.py

Lines 95 to 113 in 100d673

def display_django_heroes():
donors = DjangoHero.objects.for_public_display()
i = 0
for i, donor in enumerate(donors):
if (
donor.donated_amount is not None
and donor.donated_amount < LEADERSHIP_LEVEL_AMOUNT
):
break
return {
"corporate_members": CorporateMember.objects.by_membership_level(),
"leaders": donors[:i],
"heroes": donors[i:],
"inkind_donors": InKindDonor.objects.all(),
"display_donor_days": DISPLAY_DONOR_DAYS,
"display_logo_amount": int(LEADERSHIP_LEVEL_AMOUNT),
"corporate_membership_amounts": CORPORATE_MEMBERSHIP_AMOUNTS,
}

The fundraisers also have to have donated in the past year to show up

djangoproject.com/fundraising/models.py

Lines 25 to 35 in 5537f96

class DjangoHeroManager(models.Manager):
def for_public_display(self):
donors = (
self.get_queryset()
.filter(
is_visible=True,
approved=True,
donation__payment__date__gt=datetime.date.today()
- datetime.timedelta(days=DISPLAY_DONOR_DAYS),
)
.annotate(donated_amount=models.Sum("donation__payment__amount"))

@house-ben house-ben changed the title Update heroes donation limit text Update Leaders donation limit text Sep 11, 2025
@house-ben house-ben force-pushed the update-heroes-level-description branch from 8c8751b to 2b3f790 Compare September 11, 2025 20:31
@house-ben
Copy link
Author

I tried squashing my commits. I'm not familiar enough with git to understand how to get my two commits down to one. Anyone that can help?

CuriousLearner
CuriousLearner reviewed Sep 11, 2025
View reviewed changes
djangoproject/templates/fundraising/includes/display_django_heroes.html
Leadership-level donors contribute ${{ amount }} or
more in a calendar year.
Leadership-level donors contribute at least ${{ amount }}
and have donated in the last {{ display_donor_days }} days.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if we want to have this displaying in the last 365 days. I think I'll prefer to say in the last calendar year instead.

Suggested change
and have donated in the last {{ display_donor_days }} days.
and have donated in the last calendar year.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thibaudcolas What do you think about this? Is the intention here to say 365 days explicitly?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rechecked this, and the way we calculate this is indeed for the last 365 days. So, this change looks good to me.

CuriousLearner
CuriousLearner approved these changes Sep 11, 2025
View reviewed changes
Copy link
Member

@CuriousLearner CuriousLearner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🌮

Great work @house-ben 🎉 Congrats on your first contribution!

SaptakS
SaptakS approved these changes Sep 12, 2025
View reviewed changes
Copy link
Contributor

@SaptakS SaptakS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@house-ben thanks for the PR! The code change makes sense to me. I will let the stakeholders give the final approval for the language change and to check if this PR fulfills the requirements.

cc @sabderemane @thibaudcolas

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SaptakS SaptakS SaptakS approved these changes

+1 more reviewer

@CuriousLearner CuriousLearner CuriousLearner approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@house-ben @CuriousLearner @SaptakS